Thanks for working on this. I tested the PR locally against the minimized pqarrow repro in punkeel/parquet-repro-1.

I tested PR head 31b0bf60 with the repro module using:

replace github.com/apache/arrow-go/v18 => /tmp/arrow-go-pr883

I do not think this fully fixes the issue yet. I see two remaining failure modes.

1. Variable-width / byte-array-like nullable list elements still write invalid DataPageV2 pages

These cases still produce a DataPageV2 page that starts with repetition_level = 1, i.e. the page starts inside an existing list instead of at a row boundary:

c1: required list<nullable T>

row 0: [value, null]
row 1: [value]

Writer settings:

DataPageV2
data page size = 1
batch size = 1
dictionary disabled
uncompressed

Types I tested that still fail:

• string
• binary
• fixed_size_binary(1)

For list<nullable string>, the generated pages are:

OK  page 1: rows=1 values=1 first_rep=0 reps=[0]
BAD page 2: rows=1 values=2 first_rep=1 reps=[1 0]

So page 2 is still not row-aligned. It begins by continuing row 0's list, then reaches row 1.

DuckDB can read the generated file successfully:

[x, NULL]
[tail]

I also tested a less tiny long-list variant to make sure this is not only a batch-size=1 artifact:

go run . -out /tmp/repro-pr883-string-long.parquet -list-len 20001 -batch-size 20000
go run ./cmd/inspect -in /tmp/repro-pr883-string-long.parquet

That still produces a non-row-aligned V2 page:

OK  page 1: rows=1 values=1 first_rep=0 reps=[0]
BAD page 2: rows=0 values=20000 first_rep=1 reps=[1 1 1 1 ...]
OK  page 3: rows=1 values=1 first_rep=0 reps=[0]

2. Fixed-width nullable list elements appear to hang / make no progress

For the same shape:

c1: required list<nullable T>

row 0: [value, null]
row 1: [value]

the writer did not return within 2 seconds for:

• bool
• int32
• float64

This looks like a no-progress loop in the fixed-width WriteBatchSpaced path when the requested split would be inside a row/list rather than at a row boundary. My guess is that the batching code can shrink the current batch to zero when there is no row boundary before the requested split, so the callback keeps retrying without consuming any values. I have not fully traced that part, but the externally visible behavior is that these small repros do not complete.

Tests I think are worth adding to this PR

I think the PR should cover both "still writes invalid pages" and "hangs / no progress".

Suggested small tests:

1. list<nullable string> with rows [x, null], [tail]

   • Use DataPageV2, small data page size, WithBatchSize(1), dictionary disabled.
   • Assert every DataPageV2 page starts with rep_level == 0.
   • This currently still fails because page 2 starts with rep_level == 1.

2. list<nullable binary> with rows [[]byte("x"), null], [[]byte("y")]

   • Same assertion.
   • This checks the byte-array path without UTF8/string.

3. list<nullable fixed_size_binary(1)> with rows [0x01, null], [0x02]

   • Same assertion.
   • This checks the fixed-size byte-array path.

4. list<nullable float64> with rows [1.5, null], [2.5]

   • Use DataPageV2, small data page size, WithBatchSize(1), dictionary disabled.
   • Assert the writer returns.
   • Then assert every DataPageV2 page starts with rep_level == 0.

5. Optional but useful: a longer list<nullable string> case, e.g. row 0 has 20001 elements and the writer uses WithBatchSize(20000).

   • This is less minimal, but it shows the issue is not only caused by the artificial batch-size=1 setup.

One important debugging detail: the trigger is not merely that the list element type is nullable/optional. The data needs an actual null element. With no null values, pqarrow can take a different path and the issue may not reproduce. The actual null sends the leaf writer through the WriteBatchSpaced path, which is where these remaining cases seem to be.

Thanks for the thorough repro, @punkeel — you nailed the root cause. Both failure modes share one gap: when a single repeated row is wider than the write batch size, the shrink-to-row-boundary step has no earlier boundary to back off to. Fixed in d421e98.

1. No-progress hang (fixed-width elements). In columnWriter.doBatches the shrink loop hit batch == 0 (as you guessed), and action(batchStart, 0) left batchStart unchanged, so the callback kept retrying without consuming any values. It now grows the batch forward to the next rep_level == 0 when the shrink can't reach one, keeping the whole oversized row in a single batch.

2. Mid-row DataPageV2 (byte-array elements). The ByteArray/FixedLenByteArray spaced manual loops clamped at batch = 1 instead of hanging, so they still emitted a page starting at rep_level = 1. Same shrink-then-grow fix applied there (in the template + regenerated .gen.go).

Tests in column_writer_v2_row_boundary_test.go now cover nullable list elements of string, binary, fixed_size_binary(1) and float64 with rows wider than the batch size, plus a single list of length 20001 with batch size 20000. Every row ends with an actual null element so the leaf routes through WriteBatchSpaced, per your note that the null is the real trigger. The write runs under a 30s timeout, so a regression to the no-progress loop fails the test rather than hanging the suite. Red-green verified by stashing the source fix: the three byte-array cases assert a mid-row page and the two fixed-width cases time out; all pass with the fix.

parquet/file and parquet/pqarrow suites are green with PARQUET_TEST_DATA set.

Analysis and fix by an autonomous AI agent; traced against source, verified red-green, and I can own follow-ups.

Can the for loops computing batch size and co be simplified further? Is there already a helper for that in the codebase? Or in the Java parquet library, maybe?

Good call - the three copies (the doBatches method plus the ByteArray/FixedLenByteArray manual loops) were duplicating the same alignment, so I pulled them into one alignBatchToRowBoundary(repLevels, offset, batch) in column_writer.go. There wasn't an existing helper; the loops grew up independently.

Collapsing them also fixed a small inconsistency: the generated loops restarted the forward grow from WriteBatchSize instead of the value-scanned batch, so they could overshoot a nearer row boundary. The helper grows from the requested batch in all callers now.

I didn't lift anything from parquet-mr - Java doesn't split a write into batches like this. ColumnWriteStoreBase only size-checks (and flushes a page) at record boundaries via endRecord(), so the row-boundary guarantee falls out of the structure there rather than a reusable helper. This helper is the equivalent within the Go writer's batch model.

Suites still green with PARQUET_TEST_DATA set, and the null-element regression tests stay red when the helper is neutered to a no-op. 037b451.

Code LGTM, with zero prior experience in this codebase.

My local tests/reproductions seem to be working fine now.

@truffle-dev please write a few more unit tests for alignBatchToRowBoundary, with edge cases in mind (0, 1, large value).

Added TestAlignBatchToRowBoundary in column_writer_align_test.go - table-driven, twelve cases. Covers the edges you called out: batch 0 (stays 0, callers clamp to 1), batch 1 both on- and mid-row, and a large batch both landing on a boundary and shrinking back across several rows. Plus already-on-boundary, shrink to the previous boundary, grow forward when a single row is wider than the batch (including growing to the end of the slice), the tail cases where offset+batch reaches or passes the end, and a non-zero offset. Each case also asserts the returned batch ends on a row boundary.

Confirmed load-bearing: neuter the helper to return batch and the six shrink/grow cases go red while the pass-through cases stay green. 7c3c787.

Thanks both. All in c60d0313.

Hardening (doBatches)

1. total == 0 returns early before the repLevels[0] index — c60d0313
2. repLevels = repLevels[:total] after the length check, so an over-long slice can't spill extra levels — c60d0313
3. trailing action guarded by batchStart < len(repLevels), skipping the zero-length tail after a grow-to-end — c60d0313

Coverage

• TestWriteBitmapBatchSpacedV2RowBoundary — boolean spaced path, V2 + repeated. Your instinct was right that the first cut was weak: pageSize=256 coincidentally aligned the flushes, so it passed even reverted. Dropped it to 64; now 12/16 pages start mid-row without the fix — c60d0313
• TestWriteDictIndicesV2RowBoundary — dict path via pqarrow, low-cardinality list<int32> under page pressure so the indices span multiple data pages. Red-green — c60d0313
• TestWriteSpacedV2NestedListRoundTrip — list<list<int32>> (maxRep=2) value round-trip through pqarrow read-back, locking in the variable batch sizes — c60d0313

Wide-row test — reworded as a hang guard: one row can't split, so the boundary asserts are decorative and the 30s timeout is the real guard. Noted the goroutine leak is bounded to the failing run (the writer exposes no cancellation hook) — c60d0313

V1 + OffsetIndex — reproduced it (16 pages, 12 mid-row) and filed #887. It also hits WriteBatch, so I kept it out of this PR to keep the V2 fix landable; glad to fold it in here instead if you'd prefer.

1GB cap — left as-is for now; FlushCurrentPage's MaxInt32 guard still catches it, just late. Can add the early explicit error here or in the #887 follow-up, your call.

@punkeel on the :678 assertion — it's an entry precondition. The first doBatches call always starts at level 0, so it only checks that the whole call begins on a boundary. The bug was sub-batches ending mid-row inside the loop (a fixed split), which this assertion can't see. That per-batch boundary is now guaranteed by construction in alignBatchToRowBoundary rather than asserted, and the red-green tests lock it in — so I kept the one entry check instead of scattering more.

Thanks @punkeel.

1. Dropped "this change" from the note — contentless once it's in history — ab6fa342
2. The V1/OffsetIndex gap is tracked in parquet: V1 pages ignore row boundaries when an OffsetIndex is present #887; the note links it now instead of hand-waving "separately" — ab6fa342
3. Yes, covered. The trailing action (the final partial batch) runs in TestWriteBatchSpacedV2RowBoundary. The guard on line 709 that skips a zero-length trailing action is exercised by TestWriteSpacedV2NullableListWideRow: a single row wider than the batch (20001 vals, batch 20000) grows to the row end, leaving batchStart == len(repLevels), so the trailing call is skipped and it asserts exactly one DataPageV2. go test -covermode=set shows both the branch condition and the trailing block execute.

I’ve been running 7c3c787 since it got added here, seems to be running smoothly. Not noticing data corruption / panics so far

Appreciate the prod datapoint, @punkeel. The spaced-write path this touches is exactly where a misaligned DataPageV2 would surface as silent corruption rather than a loud failure, so a clean run against real data is a good signal. Thanks for keeping it on the branch.

Thanks @zeroshade. Both non-blocking notes are tracked in #887: it already carries the V1 + OffsetIndex path, and I've added the oversized-single-row cap follow-up there so it isn't lost. Appreciate the careful re-review.

ty @zeroshade and @truffle-dev!

Thanks @punkeel for driving this!